Fix large-message initialization in communication benchmark#1016
Fix large-message initialization in communication benchmark#1016pentschev wants to merge 2 commits intorapidsai:mainfrom
Conversation
`bench_comm` could fail while initializing input buffers for per-peer messages at around 8 GiB and larger, before the benchmark itself ran. This change allows large-message all-to-all cases to run correctly.
pentschev
added
bug
| RAPIDSMPF_EXPECTS( | ||
| nelem <= static_cast<std::size_t>(std::numeric_limits<index_t>::max()), | ||
| "random_device_vector size exceeds signed iterator range" | ||
| ); |
There was a problem hiding this comment.
Nit. Do we consider 32-bit systems? For 64 bit nelem > INT64_MAX seems unlikely isnt it?
There was a problem hiding this comment.
Nit. Do we consider 32-bit systems?
For 64 bit nelem > INT64_MAX seems unlikely isnt it?
Removed that.
Always use
safe_cast()
Done.
| } | ||
|
|
||
| std::unique_ptr<cudf::column> random_column( | ||
| cudf::size_type nrows, |
There was a problem hiding this comment.
Shouldnt this be std::size_t now to reflect the above change? Otherwise we will be clipping values above UINT32_MAX, isnt it?
There was a problem hiding this comment.
You can't make a column with more than size_type::max rows, so I think the point is moot?
There was a problem hiding this comment.
Shouldnt this be
std::size_tnow to reflect the above change? Otherwise we will be clipping values above UINT32_MAX, isnt it?
Not really, for cuDF columns/rows we should still use cudf::size_type, but that shouldn't limit us from generating/filling larger vectors (such as those we need for bench_comm that are completely unaware of cuDF).
| buffer.size / sizeof(std::int32_t) + sizeof(std::int32_t), | ||
| (buffer.size + sizeof(std::int32_t) - 1) / sizeof(std::int32_t), |
There was a problem hiding this comment.
if buffer is empty now, we will be passing 0 to random_device_vector. I think random_fill was trying to prevent that before? I'm not sure.
|
|
||
| cudf::table random_table( | ||
| cudf::size_type ncolumns, | ||
| cudf::size_type nrows, |
4 participants
bench_commcould fail while initializing input buffers for per-peer messages at around 8 GiB and larger, before the benchmark itself ran. This change allows large-message all-to-all cases to run correctly.